Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

gjs: clean up #73841

Merged
merged 1 commit into from
Nov 21, 2019
Merged

gjs: clean up #73841

merged 1 commit into from
Nov 21, 2019

Conversation

jtojnar
Copy link
Member

@jtojnar jtojnar commented Nov 20, 2019

  • Do not propagate spidermonkey, it is only needed gjs to link against.
  • Split out dev output, any propagated dependencies should go there.
  • Drop unnecessary sed, the readline thing is no longer there.

It is just 2.5 MB closure reduction but every megabyte counts.

@ofborg ofborg bot requested review from hedning and worldofpeace November 20, 2019 23:13
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100 labels Nov 20, 2019
worldofpeace
worldofpeace suggested changes Nov 21, 2019
View reviewed changes
Copy link
Contributor

@worldofpeace worldofpeace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add tests in passthru and use pname there? (not sure how I forgot this one)
Though these tests seem to timeout regularly

Also, glib things are in Requires.

@jtojnar
gjs: clean up
201c1d3 
* Do not propagate spidermonkey, it is only needed gjs to link against.
* Split out dev output, any propagated dependencies should go there.
* Drop unnecessary sed, the readline thing is no longer there.
* Add passthru.tests to allow running installed tests easily.
* Drop pango from buildInputs, we only need it as a transitive dependency of gtk3 for installed tests; gjs actually depends on cairo.
* Move libxml2 to nativeBuildInputs, it is called by glib-compile-resources when xml-stripblanks preprocessing is used.
* Propagate glib, as it is in Requires in the pc file.

It is just 2.5 MB closure reduction but every megabyte counts.
@jtojnar jtojnar requested a review from worldofpeace November 21, 2019 00:34
@jtojnar
Copy link
Member Author

jtojnar commented Nov 21, 2019

Should we add a timeout argument to nixosTests.installed-tests? Or add meta argument and merge that? Or merge everything but the explicit arguments (NixOS/rfcs#58 would help)?

Also I looked at adding sysprof-capture, but sysprof’s closure is just huge. Another candidate for scalpel.

@worldofpeace
Copy link
Contributor

Should we add a timeout argument to nixosTests.installed-tests? Or add meta argument and merge that? Or merge everything but the explicit arguments (NixOS/rfcs#58 would help)?

Well we do have testRunnerFlags which is a blanket of simplicity, but then this could turn into adding one for every option

-d, --dir                   Only run tests from these dirs (default: all system data dirs)
-l, --list                  List matching tests
-p, --parallel=PROC         Specify parallelization to PROC processors; 0 will be dynamic)
--first-root=PROC           Only use first entry in XDG_DATA_DIRS
-L, --log-directory=DIR     Create a subdirectory with test logs
--report-directory=DIR      Create a subdirectory per failing test in DIR
--status=yes/no/auto        Output status information
--log-msgid=MSGID           Log unique message with id MSGID=MESSAGE
-t, --timeout=TIMEOUT       Cancel test after timeout seconds; defaults to 5 minutes

Not sure what would be better.

Also I looked at adding sysprof-capture, but sysprof’s closure is just huge. Another candidate for scalpel.

It seems you can configure the build to just be sysprof-capture

worldofpeace
worldofpeace approved these changes Nov 21, 2019
View reviewed changes
Copy link
Contributor

@worldofpeace worldofpeace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, fixing the test is out of scope of this PR.

@jtojnar
Copy link
Member Author

jtojnar commented Nov 21, 2019

Well we do have testRunnerFlags which is a blanket of simplicity, but then this could turn into adding one for every option

This is Hydra timing out so we would need to change meta.timeout.

@jtojnar jtojnar merged commit b94b8ca into NixOS:master Nov 21, 2019
@jtojnar jtojnar deleted the gjs-cleanup branch November 21, 2019 01:05
@jtojnar
Copy link
Member Author

jtojnar commented Nov 21, 2019

Would not it be built twice then?

@jtojnar jtojnar mentioned this pull request Nov 22, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@worldofpeace worldofpeace worldofpeace approved these changes

@hedning hedning Awaiting requested review from hedning

Assignees
No one assigned
Labels
10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jtojnar @worldofpeace